Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add remaining Python 3 builtins via six #1700

Merged
merged 2 commits into from
Jul 7, 2015
Merged

Add remaining Python 3 builtins via six #1700

merged 2 commits into from
Jul 7, 2015

Conversation

QuLogic
Copy link
Member

@QuLogic QuLogic commented Jun 12, 2015

To follow up on #1699, this adds the compatibility builtins on all files. This will look bigger because it's based on #1699, but I want to be sure tests can pass here too.

Much like the __future__ imports, this PR adds six imports that change behaviour of builtin functions. This is a blanket change, so hopefully it's not too hard to go through again. At least this time there's a functional change involved.

@QuLogic QuLogic force-pushed the py3k-six-global-builtin branch 2 times, most recently from 410a278 to ca1a5d7 Compare June 13, 2015 05:21
@QuLogic QuLogic mentioned this pull request Jun 13, 2015
Merged
@QuLogic QuLogic force-pushed the py3k-six-global-builtin branch from ca1a5d7 to 6c02f5d Compare June 22, 2015 21:56
@QuLogic QuLogic mentioned this pull request Jun 24, 2015
Merged
@QuLogic
Copy link
Member Author

QuLogic commented Jun 24, 2015

New tests are being added that do not use this new behaviour; please merge soon.

@rhattersley
Copy link
Member

Looks like there are some unwanted commits on this branch as it stands.

@QuLogic
Copy link
Member Author

QuLogic commented Jun 25, 2015

I did a merge instead of a rebase to show the changes to the new files, but I'm not sure why the new commits from master are now showing as part of the PR.

@QuLogic
Copy link
Member Author

QuLogic commented Jun 25, 2015

GitHub is being weird, it seems, because I see only three new commits locally:

$ git cherry master py3k-six-global-builtin 
+ d517878ab73b27dbe3ef2fbc578fc5b149bbf556
+ 6c02f5d011e0d73868f34f109943cb0f5d63d28c
+ 537e246484708b1a58cafcd65d87603553dca9c3

@QuLogic QuLogic mentioned this pull request Jun 25, 2015
Merged
@QuLogic QuLogic force-pushed the py3k-six-global-builtin branch from e094dd0 to e164a71 Compare July 1, 2015 06:57
@QuLogic
Copy link
Member Author

QuLogic commented Jul 1, 2015

This is now flat again. BTW, you can check that it only changes what I say it changes like this:

$ git log -1 -p a552c3e | grep '^-' | grep -v '^---' | sort -u
-# (C) British Crown Copyright 2010 - 2014, Met Office
-# (C) British Crown Copyright 2012 - 2014, Met Office
-# (C) British Crown Copyright 2013 - 2014, Met Office
-# (C) British Crown Copyright 2014, Met Office
-copyright = u'British Crown Copyright 2010 - 2014, Met Office'
-from six.moves import filter
-from six.moves import filter, map, range
-from six.moves import filter, map, range, zip
-from six.moves import filter, range
-from six.moves import filter, range, zip
-from six.moves import filter, zip
-from six.moves import map
-from six.moves import map, range, zip
-from six.moves import map, zip
-from six.moves import range
-from six.moves import range, zip
-from six.moves import zip

$ git log -1 -p a552c3e | grep '^+' | grep -v '^+++' | sort -u
+# (C) British Crown Copyright 2010 - 2015, Met Office
+# (C) British Crown Copyright 2012 - 2015, Met Office
+# (C) British Crown Copyright 2013 - 2015, Met Office
+# (C) British Crown Copyright 2014 - 2015, Met Office
+copyright = u'British Crown Copyright 2010 - 2015, Met Office'
+from six.moves import (filter, input, map, range, zip)  # noqa

@pelson
Copy link
Member

pelson commented Jul 1, 2015

To follow up on #1699, this adds the compatibility builtins on all files

I thought we'd decided not to do this? I'm not that keen on the change, though wouldn't block it. Just to be completely clear, are all of these changes necessary, or are they done for consistency with all other source files?

@QuLogic
Copy link
Member Author

QuLogic commented Jul 1, 2015

No, I think you're thinking of #1675 which added six imports but had no real impact on anything. This PR, on the other hand, imports the builtins that were changed in Python 3. So, much like the __future__ imports, it ensures consistency across 2 and 3.

@QuLogic QuLogic mentioned this pull request Jul 6, 2015
Merged
@QuLogic QuLogic force-pushed the py3k-six-global-builtin branch from e164a71 to dcc08ad Compare July 6, 2015 21:47
@QuLogic QuLogic force-pushed the py3k-six-global-builtin branch from dcc08ad to 7f4d1b5 Compare July 7, 2015 03:27
@rhattersley
Copy link
Member

from six.moves import (filter, input, map, range, zip) # noqa

As a reminder to myself, the noqa tag stops flake8 from complaining about the unused imports.

@rhattersley
Copy link
Member

👍 Top marks for persistence @QuLogic!

rhattersley added a commit that referenced this pull request Jul 7, 2015
@rhattersley
Merge pull request #1700 from QuLogic/py3k-six-global-builtin
00a168f 
Add remaining Python 3 builtins via six
@rhattersley rhattersley merged commit 00a168f into SciTools:master Jul 7, 2015
@ajdawson ajdawson mentioned this pull request Jul 7, 2015
Merged
@QuLogic QuLogic deleted the py3k-six-global-builtin branch July 7, 2015 21:07
@QuLogic
Copy link
Member Author

QuLogic commented Jul 7, 2015

👍 Top marks for persistence @QuLogic!

That and some patience! 😉

@QuLogic QuLogic added this to the v1.9.0 milestone Jul 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.9.0
Development

Successfully merging this pull request may close these issues.
3 participants
@QuLogic @rhattersley @pelson